Add graph extraction helpers to make it easier to use NVL #517
Conversation
|
anderson4j
left a comment
anderson4j
left a comment
There was a problem hiding this comment.
Looks good. I had some small comments, but after you decide on those it should be fine to merge
packages/query-tools/src/cypher-execution/extract-unique-nodes-and-relationships.test.ts
Outdated
Show resolved
Hide resolved
| /** | ||
| * Result type containing deduplicated nodes and relationships extracted from Neo4j records. | ||
| */ | ||
| export type DeduplicatedBasicNodesAndRels = { |
There was a problem hiding this comment.
What does the "Basic" part here mean?
There was a problem hiding this comment.
Orginally we named the nodes BasicNode to avoid clashing with the web browser api Node and to distinguish from the Nodes used in NVL. Here we don't have that problem, so I'll drop the basic part for now
| }: { nodeLimit?: number; keepDanglingRels?: boolean } = {}, | ||
| ): DeduplicatedBasicNodesAndRels => { | ||
| let limitHit = false; | ||
| if (records.length === 0) { |
There was a problem hiding this comment.
This check looks like it isnt needed. Do you think it makes the code clearer/gives a worthwhile optimization, or what do you think about removing it and instead having a test like:
test('should not break on empty bolt records', () => {
const { nodes, relationships, limitHit } = extractUniqueNodesAndRels([]);
if (
nodes === undefined ||
relationships === undefined ||
limitHit === undefined
) {
throw new Error('Invalid return values on empty array of paths');
}
expect(nodes.length).toBe(0);
expect(relationships.length).toBe(0);
expect(limitHit).toBe(false);
});
There was a problem hiding this comment.
I added a test similar to this now, thanks 👍
…-and-relationships.test.ts Co-authored-by: Isak Nilsson <[email protected]>
No description provided.